Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

• title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

@mazenkhalil thanks for this PR.

Currently, the connection is created by Liquibase because there is no public API to give Liquibase a Mongo Client, you hack a way to do this by creating new MongoLiquibaseDatabase and MongoLiquibaseConnection, I don't know the internal of Liquibase so I'm not very compfortable with the choice. By the way, I'm not sure it worth it as Liquibase will open a single connection, do the migration, then close it. Re-using our Mongo Client will not brings a big advantage here.

Moreover, the client is now retrieved from Arc at Liquibase creation, this will create it eagerly and would change current behavior. This may be an issue.

I agree that supporting multiple Mongo client should be done. But I'm more reluctant for the change that reuse the Mongo Client.

@loicmathieu Well, MongoLiquibaseDatabase and MongoLiquibaseConnection are the actual implementation for Database and Connection from Liquibase core. The only functionality there is creating MongoClient and MongoDatabase objects.

The reason why I am proposing this PR is to solve the following:

• With the current implementation, if you enable cleanAtStart, validateAtStart and migrateAtStart, 3 DB connections get opened and none get closed (ref. LiquibaseMongodbRecorder#doStartActions).
• Using the programatic method utilizing LiquibaseMongodbFactory on startup, do the job then close the connection took somewhere like 25 seconds for the app to start compares to 5 seconds without closing the connection.
• Currently there is no direct API to run migration scripts against multiple DB's. Only one DB and one changeLog file supported. The only workaround here is to access Liquibase API directly which I don't think is a clean way for implementations.

I can modify the implementation in a way to inject MongoClient when the migration actually starts, this way we can prevent the eager initialization. What do you think?

• With the current implementation, if you enable cleanAtStart, validateAtStart and migrateAtStart, 3 DB connections get opened and none get closed (ref. LiquibaseMongodbRecorder#doStartActions).

Maybe this should be reported as a bug in the Liquibase MongoDB repo.

• Using the programatic method utilizing LiquibaseMongodbFactory on startup, do the job then close the connection took somewhere like 25 seconds for the app to start compares to 5 seconds without closing the connection.

You means millisecondes not seconds right ? Anyway it's not very good yes.

• Currently there is no direct API to run migration scripts against multiple DB's. Only one DB and one changeLog file supported. The only workaround here is to access Liquibase API directly which I don't think is a clean way for implementations.

I think we add the created Liquibase object inside the CDI container but I agree multiple DBs should be handled natively.

I can modify the implementation in a way to inject MongoClient when the migration actually starts, this way we can prevent the eager initialization. What do you think?

Yes, it would be great.

• With the current implementation, if you enable cleanAtStart, validateAtStart and migrateAtStart, 3 DB connections get opened and none get closed (ref. LiquibaseMongodbRecorder#doStartActions).

Maybe this should be reported as a bug in the Liquibase MongoDB repo.

The problem is from the recorder itself where 3 Liquibase instances get created (New connection/Liquibase instance) and none get closed.

• Using the programatic method utilizing LiquibaseMongodbFactory on startup, do the job then close the connection took somewhere like 25 seconds for the app to start compares to 5 seconds without closing the connection.

You means millisecondes not seconds right ? Anyway it's not very good yes.

My bad. It was actually taking seconds as I was running multiple migration scripts against multiple databases sequentially. For some reason most of the time was coming from closing the connection.

I've made the required changes and squashed the commits.

I think the behavious changed. Before we can use liquibase with a database name inside the connection string (we had a regex that check that if the database is not in the connection string it must be specified in the config), now we mandate a database inside the config.

You are right. What do you think about proposing a new property under liquibase for that? I think it is cleaner, and we can ensure backward compatibility as follows:

1. Liquibase configuration (e.g. quarkus.liquibase.database)
2. If quarkus.liquibase.database is not defined, check mongoclient database property (quarkus.mongodb.database)
3. If quarkus.mongodb.database is not defined, check the connection string to get the DB name
4. If no DB name defined within the connection string, throw an error.

@mazenkhalil I'm not sure we need two properties, one for the MongoDB client/panache extension and one for the Liquibase MongoDB extension. What's the use case for different propoperties ?

@mazenkhalil I'm not sure we need two properties, one for the MongoDB client/panache extension and one for the Liquibase MongoDB extension. What's the use case for different propoperties ?

This would allow running two different migration scripts against different databases utilizing same MongoClient.

This would allow running two different migration scripts against different databases utilizing same MongoClient.

For this we would need the ability to have multiple liquibase configuration. Maybe the best is to make these enhancements steps by steps : first allow to reuse the client and use a named client, then allow multiple liquibase.

For this we would need the ability to have multiple liquibase configuration. Maybe the best is to make these enhancements steps by steps : first allow to reuse the client and use a named client, then allow multiple liquibase.

Agree. I've added back the regex to parse the connection string if no database defined in the config; but I changed the order. Previously the config property was used if no database defined in the connection string, now we fallback to the connection string when no database config property is defined (I think it is more proper this way as we use mongo client now and its properties should have the priority).

I will introduce another PR soon to allow multi liquibase migrations.

@mazenkhalil I extracted the part where you avoid to create 3 LiquibaseFactory in #30778 as I think we should merge this ASAP (and backport it).

As for the rest, I really think we should try to focus on an approach similar to what is done in the standard Liquibase extension. Would you be willing to give it a try?

@mazenkhalil I extracted the part where you avoid to create 3 LiquibaseFactory in #30778 as I think we should merge this ASAP (and backport it).

As for the rest, I really think we should try to focus on an approach similar to what is done in the standard Liquibase extension. Would you be willing to give it a try?

I will look into it later this week.

Closing for lack of activity and conflicts.

I think part of the issues have been addressed (like the fact that connections are closed) but maybe not all of them.